`rustc_borrowck` cleanups #132250

nnethercote · 2024-10-28T05:22:29Z

A bunch of cleanups I made while reading over this crate.

r? @lqd

compiler/rustc_borrowck/src/lib.rs

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

compiler/rustc_borrowck/src/diagnostics/region_name.rs

nnethercote · 2024-10-29T03:42:17Z

I've removed the commits you didn't like, should be good to go now.

voidc · 2024-10-29T08:48:45Z

Reducing the visibility of items in this crate might break tools using the get_body_with_borrowck_facts API. In #111840, some types and methods were deliberately made public, so that they could be used externally.

nnethercote · 2024-10-29T20:09:49Z

visibility of items

I checked. Nothing in the consumers.rs file was changed. I thought that file contained the entire public API, but I found there are a few items outside that file marked with a comment as being in the API. I have reverted the visibility changes to those items. Should be good to go now, thanks.

lqd · 2024-10-29T20:48:53Z

as they’ve already started reviewing, r? compiler-errors

bors · 2024-10-30T11:20:42Z

☔ The latest upstream changes (presumably #132349) made this pull request unmergeable. Please resolve the merge conflicts.

nnethercote · 2024-10-31T04:26:18Z

@compiler-errors: I rebased, ready for review again, thanks.

compiler-errors · 2024-11-03T22:10:05Z

r=me after rebasing

So they're all in the one place. Also prepend with `crate::`, à la the `unqualified_local_imports` lint.

Mostly by wrapping overly long comment lines, plus a few other things.

It's strange to have a struct that contains a single anonymous field that is an enum. This commit merges them. This does require increasing the visibility of `TypeOfInfo` to `pub(crate)`, but that seems worthwhile.

It's unused.

Because there is no real reason for it to be a separate struct. - It has no methods. - It's easy to confuse with the nearby `BorrowckInferContext` (which does have methods). - The `mut` ref to it in `TypeChecker` makes it seem like any of the fields within might be mutable, but only two (`all_facts` and `constraints`) actually are. - Two of the fields are `pub(crate)` but can be private. This change makes a lot of code more concise and readable.

It has four different `insert` methods, with some duplication. This commit finds the commonality and removes them all.

- Store a mut ref to a `BorrowckDiags` in `MirBorrowckCtxt` instead of owning it, to save having to pass ownership in and out of `promoted_mbcx`. - Use `buffer_error` in a couple of suitable places.

nnethercote · 2024-11-04T06:37:45Z

@bors r=compiler-errors

bors · 2024-11-04T06:37:48Z

📌 Commit e0e7a43 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-11-04T09:19:08Z

⌛ Testing commit e0e7a43 with merge ca87b53...

bors · 2024-11-04T12:11:48Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing ca87b53 to master...

rust-timer · 2024-11-04T13:29:53Z

Finished benchmarking commit (ca87b53): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 779.582s -> 779.908s (0.04%)
Artifact size: 335.21 MiB -> 335.29 MiB (0.03%)

…2, r=<try> `rustc_borrowck` cleanups, part 2 The code under `do_mir_borrowck` is pretty messy, especially the various types like `MirBorrowckCtxt`, `BorrowckInferCtxt`, `MirTypeckResults`, `MirTypeckRegionConstraints`, `CreateResult`, `TypeChecker`, `TypeVerifier`, `LivenessContext`, `LivenessResults`. This PR does some tidying up, though there's still plenty of mess left afterwards. A sequel to rust-lang#132250. r? `@compiler-errors`

…2, r=Nadrieril `rustc_borrowck` cleanups, part 2 The code under `do_mir_borrowck` is pretty messy, especially the various types like `MirBorrowckCtxt`, `BorrowckInferCtxt`, `MirTypeckResults`, `MirTypeckRegionConstraints`, `CreateResult`, `TypeChecker`, `TypeVerifier`, `LivenessContext`, `LivenessResults`. This PR does some tidying up, though there's still plenty of mess left afterwards. A sequel to rust-lang#132250. r? `@compiler-errors`

rustbot assigned lqd Oct 28, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 28, 2024

compiler-errors requested changes Oct 28, 2024

View reviewed changes

compiler/rustc_borrowck/src/lib.rs Outdated Show resolved Hide resolved

compiler/rustc_borrowck/src/type_check/free_region_relations.rs Outdated Show resolved Hide resolved

compiler/rustc_borrowck/src/diagnostics/region_name.rs Outdated Show resolved Hide resolved

nnethercote force-pushed the rustc_borrowck-cleanups branch from f3ad954 to 41c968e Compare October 29, 2024 03:40

nnethercote marked this pull request as ready for review October 29, 2024 03:41

nnethercote force-pushed the rustc_borrowck-cleanups branch from 41c968e to b4fca11 Compare October 29, 2024 20:06

rustbot assigned compiler-errors and unassigned lqd Oct 29, 2024

nnethercote force-pushed the rustc_borrowck-cleanups branch from b4fca11 to 0bcd936 Compare October 31, 2024 04:25

nnethercote requested a review from compiler-errors November 3, 2024 19:49

compiler-errors approved these changes Nov 3, 2024

View reviewed changes

nnethercote added 10 commits November 4, 2024 15:43

Reduce visibilities.

6676cec

Move some use declarations.

ad87552

So they're all in the one place. Also prepend with `crate::`, à la the `unqualified_local_imports` lint.

Tidy up comments and some formatting.

bf1a5c2

Mostly by wrapping overly long comment lines, plus a few other things.

Merge UniverseInfo and UniverseInfoInner.

7901c03

It's strange to have a struct that contains a single anonymous field that is an enum. This commit merges them. This does require increasing the visibility of `TypeOfInfo` to `pub(crate)`, but that seems worthwhile.

Remove ToUniverseInfo impl for CanonicalQueryInput<CustomTypeOp>.

e23bdd6

It's unused.

Remove unnecessary continue.

a07011b

Remove unnecessary qualifiers.

f86c76a

Simplify LocalUseMapBuild.

ce2f0b4

It has four different `insert` methods, with some duplication. This commit finds the commonality and removes them all.

BorrowckDiags tweaks.

e0e7a43

- Store a mut ref to a `BorrowckDiags` in `MirBorrowckCtxt` instead of owning it, to save having to pass ownership in and out of `promoted_mbcx`. - Use `buffer_error` in a couple of suitable places.

nnethercote force-pushed the rustc_borrowck-cleanups branch from 0bcd936 to e0e7a43 Compare November 4, 2024 06:36

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 4, 2024

bors merged commit ca87b53 into rust-lang:master Nov 4, 2024
7 checks passed

rustbot added this to the 1.84.0 milestone Nov 4, 2024

This was referenced Nov 4, 2024

Migrate compiler's &Option<T> into Option<&T> #130963

Closed

Remove ResultsVisitable #132134

Merged

nnethercote deleted the rustc_borrowck-cleanups branch November 4, 2024 22:56

nnethercote mentioned this pull request Nov 5, 2024

rustc_borrowck cleanups, part 2 #132623

Merged

nilehmann mentioned this pull request Nov 28, 2024

Update toolchain flux-rs/flux#913

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`rustc_borrowck` cleanups #132250

`rustc_borrowck` cleanups #132250

Uh oh!

nnethercote commented Oct 28, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nnethercote commented Oct 29, 2024

Uh oh!

voidc commented Oct 29, 2024

Uh oh!

nnethercote commented Oct 29, 2024

Uh oh!

lqd commented Oct 29, 2024

Uh oh!

bors commented Oct 30, 2024

Uh oh!

nnethercote commented Oct 31, 2024

Uh oh!

compiler-errors commented Nov 3, 2024

Uh oh!

nnethercote commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

Uh oh!

rust-timer commented Nov 4, 2024

Uh oh!

Uh oh!

rustc_borrowck cleanups #132250

rustc_borrowck cleanups #132250

Uh oh!

Conversation

nnethercote commented Oct 28, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nnethercote commented Oct 29, 2024

Uh oh!

voidc commented Oct 29, 2024

Uh oh!

nnethercote commented Oct 29, 2024

Uh oh!

lqd commented Oct 29, 2024

Uh oh!

bors commented Oct 30, 2024

Uh oh!

nnethercote commented Oct 31, 2024

Uh oh!

compiler-errors commented Nov 3, 2024

Uh oh!

nnethercote commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

bors commented Nov 4, 2024

Uh oh!

Uh oh!

rust-timer commented Nov 4, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

`rustc_borrowck` cleanups #132250

`rustc_borrowck` cleanups #132250